-
Notifications
You must be signed in to change notification settings - Fork 169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MWPW-142935 [Project PEP] PEP <> Geo Routing Modal Interactions #2403
Conversation
… shown or b) GRM is dismissed
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2403 +/- ##
==========================================
- Coverage 95.64% 95.62% -0.02%
==========================================
Files 173 173
Lines 45306 45321 +15
==========================================
+ Hits 43331 43338 +7
- Misses 1975 1983 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if you can add some unit tests for this
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
@@ -35,6 +35,16 @@ const getIcon = (content) => { | |||
return icons.company; | |||
}; | |||
|
|||
const waitForClosedGRMThen = (loadPEP) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of polling the DOM, you should listen for milo:modal:closed on the window object.
Whent that event is triggered you could then check if .locale-modal-v2 is no longer present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that the global-navigation.js/webapp-prompt.js might not have loaded by the time the user gets rid of the GRM, especially on slow connections, so the event would be missed and we would have to poll anyway unless we set a global flag. I'm not sure this would be all that much more readable than what we have now. Is there a particular compunction we should have against polling for the GRM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can check to see if .locale-modal-v2 is present in the DOM first and if it's available there, then add an event listening for it to close.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in general polling is worse for performance than just listening to the right event.
@@ -137,27 +137,34 @@ export default async function loadGeoRouting(config, createTag, getMetadata) { | |||
const storedLocale = storedInter === 'us' ? '' : storedInter; | |||
|
|||
const resp = await fetch(`${config.contentRoot ?? ''}/georouting.json`); | |||
if (!resp.ok) return; | |||
if (!resp.ok) return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all these changes are unnecessary
libs/utils/utils.js
Outdated
getMetadata, | ||
loadBlock, | ||
loadStyle, | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to make all these changes to the georouting files.
You're already awaiting a response here .
Just check if locale-modal-v2 has been added to the DOM right here
document.querySelector('.locale-modal-v2')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually in retrospect I don't even think you need to have any code here. Just add that listener I described in your webapp-prompt file, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we're cooking!
That's looking pretty good. I think you could simplify your code a little more to be sending less bytes to the client if you wanted, but now it's just nitpicking.
Very nice :)
@@ -43,8 +50,8 @@ class AppPrompt { | |||
this.getAnchorState = getAnchorState; | |||
this.id = this.promptPath.split('/').pop(); | |||
this.elements = {}; | |||
|
|||
this.init(); | |||
if (geoRoutingActive()) waitForClosedGRMThen(this.init); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (geoRoutingActive()) waitForClosedGRMThen(this.init); | |
if (geoRoutingActive()) document.addEventListener('milo:modal:closed', loadPEP); |
|| !!document.querySelector('.locale-modal'); | ||
|
||
const waitForClosedGRMThen = (loadPEP) => { | ||
window.addEventListener('milo:modal:closed', loadPEP); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for doing this, but I just realized that it's possible for two dialogs to be open.
see this link - https://stage--cc--adobecom.hlx.page/products/photoshop/ai?milolibs=pep-georouting&skipPepEntitlements=true#langnav
we need to add a check here that geoRoutingActive() is now false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've discovered that when the event is emitted it won't necessarily trigger PEP's init
method, since it's possible for the modal to still exist in the DOM at that point. I'm looking into why this might be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
completely agree.
In my opinion PEP should not be appearing if any modal is open, but since that makes your task more complicated I didn't mention it.
Maybe you can mention that to your PdM and if they deem it necessary then tackle that in a separate task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh and btw, you want to clean up your listener once you've triggered it once.
https://dev.to/js_bits_bill/addeventlistener-once-js-bits-565d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it seems a bit implementation dependent (which means the whatwg spec doesn't seem to have information about this, but there is a chromium design document here), but on chrome once the event is dispatched it seems that the eventListener's callback is called first, and then the rest of the code that removes the modal fires (kind of like a delimited continuation). See the video below:
eb.mp4
I believe the behavior that targets specifically the GRM (or perhaps modals in general?) with a custom event might be better in this case, but this would involve touching a lot more code.
In the most general sense, it's not possible for me to predict if sometime between the moment a modal is closed and the moment where the user is redirected by pep a modal will pop up (as lang nav pops up very late). So I'm willing to compromise to allowing PEP to begin iff there are no modals on the screen, while making no guarantees about future modals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sharmrj it seems to me that the right solution would be to change the modal.js so that the close event is sent after removing the modal.
@JasonHowellSlavin can you weigh in if you see issues with moving the window.dispatchEvent of milo:modal:closed after removing the modal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @vhargrave, I do think both you and @sharmrj have identified a key issue with the milo:modal:closed
dispatch being placed too high in the function and not reflecting what is actually occurring. I don't see an issue with moving it down, but since there are a few other files that rely on it, it might be good to raise that as an issue and specifically test for those implementations.
It also looks like @sharmrj has done some experimentation and found the performance impact of the current code to be negligible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JasonHowellSlavin ok, thanks for confirming 👍
@vhargrave After some experimentation I believe it would be less bad to go with a pull (polling) approach over a push approach (events), simply because it allows us to reduce the surface area of the change. As described by my earlier comment and the video, the modal is not closed when the While in many situations polling can cause a performance impact (e.g. polling multiple services for relatively rare changes), in this particular situation:
|
I do see the point regarding performance, but for me it's also about architecture. If there's an event that one can listen to then it's better to listen for an event than to poll. Even kicking off polling after the event happens is better than polling from the get go. |
If that's acceptable I can change the implementation to start polling only after the event is emitted. |
@sharmrj for me it's not perfect, but it's good enough yeah. And it means that you don't need to make any changes to the event emit that would require thorough testing. 👍 |
…al dialog comes through
@vhargrave Cool thanks for all the great feedback. Could you take one more look at the code? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my side 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the reverse a concern as well? As in: PEP prompt is open, but the georouting modal pops up. The user wouldn't be able to dismiss the PEP prompt and would get redirected, potentially causing confusion. It might be valuable to pause the PEP timer or find some alternative solutions.
if (modalsActive()) { | ||
window.addEventListener( | ||
'milo:modal:closed', | ||
() => waitForClosedModalsThen(this.init), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we not add waitForClosedModalsThen
as a class method and avoid passing the init
method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like to be explicit about what I'm passing to functions, and I like the CPS-ish semantics of writing "do an action and then do this - " by explicitly passing the function (so long as we aren't doing long chains of it, of course). Though if you think the init
method shouldn't be passed I can change it (my reasoning for why it's ok is that it's not a private method; anyone can call it).
|
||
const waitForClosedModalsThen = (loadPEP) => { | ||
if (modalsActive()) { | ||
setTimeout(() => waitForClosedModalsThen(loadPEP), 200); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we not use a setInterval
to avoid the recursive calls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to not use setInterval since we'd have to keep track of the setInterval
ID and clean it up after with clearInterval
. And waitForClosedModalsThen
isn't recursive in the sense that it's growing the callstack indefinitely, it, like setInterval
, adds a function to the task queue, and in that sense is (to us, at least) indistinguishable in behavior from setInterval
. The question is of whether we prefer the extra code to clean up the interval or the extra code for adding to the event queue.
We don't need to worry about the PEP modal being loaded before the GRM, because we have an |
Validation done on the pr in below url : |
Validation done on below feature url : |
commit a8ea3cd Author: Raghav Sharma <[email protected]> Date: Wed Jul 10 23:29:27 2024 +0530 MWPW-154013 PEP prompt redirection is broken in stage after the PEP dismissal PR merge (#2547) * Added a way to mock entitlements in non prod environments for testing purposes * Added a pulsing animation after dismissing the PEP modal * Added the ability to debug pep in prod; added skipPepEntitlements option * fixed url for loading the pep dismissal animation * Another url fix for the pep dismissal animation css file * slowed down the default animation * Added the tooltip * re-enabled tracking dismissed prompts * added tooltip that uses data attributes; cleaned up ring animation slightly * styled the tooltip in accordance with the new figma spec sheet * Changed the color of the tooltip * Pick up pep dismissal action config from section metadata; fixed lowercase url issue * Updated unit tests * updated an incomplete pep test * Fixed dismissal actions firing when redirecting * Removed a comment * added tests for running dismissal actions * Tooltip and animation are now cleared if the app switcher is clicked * added a missing semicolon * small refactor of remove animation/tooltip on click logic * A bit of cleanup * removed some unnecessary code * Fixed a bug introduced by one of the previous fixes * fixed a classname * Formatting of an html string * replaced all usage of right in absolute positioning with left * Added CONFIG default values to dismissal action function parameter lists * renamed a variable * Removed an unused variable * Grouped together common css rules in tooltip.css * Combined dismissal css with the general webapp-prompt css * Removed an unused import * Added a unit test * Removed some unneeded newlines * Moved the tooltip down slightly * Added a missing curly brace * added a missing semicolon on line 92 of libs/features/webapp-prompt/webapp-prompt.js Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * made the dismissal config its own entity * added some variables to the focus animation css * removed a redundant style * cleaned up the padding css for the tooltip * minor refactoring * Changed the tooltip fontfamily to use the milo styles variable defined in root * Removed an unnecessary css rule * Replaced tabs with spaces in webapp-prompt.css * Fixed the PEP unit tests * clock cleanup in tests * fixed an issue with the redirect * small change * Fixed eslint error by making a method static * Fixed failing tests * Fixed an issue where the redirect wasn't happening * Added a newline to get Franklin to pick up the new css * Removing the newline added previously * Fixing the tests --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit f4e3b65 Author: Ilyas Türkben <[email protected]> Date: Wed Jul 10 19:59:20 2024 +0200 MWPW-153962: Introduce maslibs query parameter (#2544) * MWPW-153962: Introduce maslibs query parameter Similar to milolibs query parameter, when present, it will load commerce.js from the external M@S repository. * update comment * remove only * improve cov + fix promise issue * retore tests * revert change in head.html will raise a second PR only for this change * update port to avoid conflicts * fix test * update tests * restrict maslibs to lower envs + stage * restrict maslibs to lower envs + stage * update /lib to /libs * update /lib to /libs * update comments commit 5b64cf1 Author: Bandana Laishram <[email protected]> Date: Wed Jul 10 23:29:12 2024 +0530 Central georouting support (#2531) * Adding central georouting support from federal repo * Unit test case Update * Unit test case Update * Remove console * Creating a helper file for feds utils function * Removing blog from default allowed list * Updating georouting fetch calls * Lint fix * Test case update * Renaming fedshelper file * Updating test file commit 7698b38 Author: Rares Munteanu <[email protected]> Date: Wed Jul 10 19:59:05 2024 +0200 [MWPW-152278] Avoid empty CSS requests (#2524) * [MWPW-152278] Avoid empty CSS requests * [MWPW-152278] Empty CSS requests PR feedback commit fb2fe67 Author: Brandon Marshall <[email protected]> Date: Wed Jul 10 10:58:58 2024 -0700 MWPW-152918 Fix Marketo button font (#2513) commit a77e493 Author: Saloni Jain <[email protected]> Date: Tue Jul 9 12:10:04 2024 +0530 Revert "MWPW-141022 [Project PEP] Prompt Dismissal + Tie-in with App Launcher UX" (#2546) Revert "MWPW-141022 [Project PEP] Prompt Dismissal + Tie-in with App Launcher…" This reverts commit 15f8c9f. commit 8e6ca69 Author: Ilyas Türkben <[email protected]> Date: Mon Jul 8 14:17:55 2024 +0200 Revert "MWPW-152283: [TEST] switch WCS to acom domain" (#2534) Revert "MWPW-152283: [TEST] switch WCS to acom domain (#2504)" This reverts commit 5e2dd4c. commit 15f8c9f Author: Raghav Sharma <[email protected]> Date: Mon Jul 8 17:47:48 2024 +0530 MWPW-141022 [Project PEP] Prompt Dismissal + Tie-in with App Launcher UX (#2392) * Added a way to mock entitlements in non prod environments for testing purposes * Added a pulsing animation after dismissing the PEP modal * Added the ability to debug pep in prod; added skipPepEntitlements option * fixed url for loading the pep dismissal animation * Another url fix for the pep dismissal animation css file * slowed down the default animation * Added the tooltip * re-enabled tracking dismissed prompts * added tooltip that uses data attributes; cleaned up ring animation slightly * styled the tooltip in accordance with the new figma spec sheet * Changed the color of the tooltip * Pick up pep dismissal action config from section metadata; fixed lowercase url issue * Updated unit tests * updated an incomplete pep test * Fixed dismissal actions firing when redirecting * Removed a comment * added tests for running dismissal actions * Tooltip and animation are now cleared if the app switcher is clicked * added a missing semicolon * small refactor of remove animation/tooltip on click logic * A bit of cleanup * removed some unnecessary code * Fixed a bug introduced by one of the previous fixes * fixed a classname * Formatting of an html string * replaced all usage of right in absolute positioning with left * Added CONFIG default values to dismissal action function parameter lists * renamed a variable * Removed an unused variable * Grouped together common css rules in tooltip.css * Combined dismissal css with the general webapp-prompt css * Removed an unused import * Added a unit test * Removed some unneeded newlines * Moved the tooltip down slightly * Added a missing curly brace * added a missing semicolon on line 92 of libs/features/webapp-prompt/webapp-prompt.js Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * made the dismissal config its own entity * added some variables to the focus animation css * removed a redundant style * cleaned up the padding css for the tooltip * minor refactoring * Changed the tooltip fontfamily to use the milo styles variable defined in root * Removed an unnecessary css rule * Replaced tabs with spaces in webapp-prompt.css * Fixed the PEP unit tests * clock cleanup in tests * fixed an issue with the redirect * small change * Fixed eslint error by making a method static * Fixed failing tests --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit 5f2f3e2 Author: Mira Fedas <[email protected]> Date: Mon Jul 8 11:15:58 2024 +0200 MWPW-153657: Set the strikethrough price font size to 14px only in merch cards (#2527) * set the strikethrough price to 14px in merch card * updated merch-card commit 14af634 Author: Amol Anand <[email protected]> Date: Mon Jul 8 05:15:51 2024 -0400 Exposing the UNav Config for consumers to modify and reload the UNav if needed (#2501) * exporting the unav config to be consumed and modified later by consumers * Adding a comment to explain why we need this * Updating comment * getting the Unav config from the object before reloading commit daef70b Author: Siva S <[email protected]> Date: Mon Jul 8 14:45:44 2024 +0530 MWPW-147468 - Delayed Activation of Blade Animations on Ps Product Page (#2487) * feat(video): implemented video play within the viewport * chore(video): opacity removed * fix: window extension * fix(DOM-exception): fixed dom exception & fixes test cases * fix(unit-test): removed dublicates urls in mocks * fix: test case added for play/pause & end event * chore: removed dublicate checks commit 265838f Author: sharathkannan <[email protected]> Date: Mon Jul 8 14:45:37 2024 +0530 Fix(MWPW-152290):Removed the horizontal scroll seen in windows machines. (#2471) * horizontal overflow are hidden * changed the rule for property * changed % to vh commit f5b5121 Author: Mira Fedas <[email protected]> Date: Thu Jun 27 22:09:35 2024 +0200 Revert "MWPW-148002: Adjust Strike-through price font size for: merch-card (all variations)" (#2529) Revert "MWPW-148002: Adjust Strike-through price font size for: merch-card (a…" This reverts commit 5ad4b21. commit 43d351f Author: Gautam Bajaj <[email protected]> Date: Thu Jun 27 17:46:56 2024 +0530 Integrating spectrum Toast in milo required by Event creation console. (#2511) * Adding Picker SWC component * Adding swc picker * updating all to latest version * Reactive controller fix * Adding support for toast * Add Chevron icon * Update to latest main version * Adding UT for Toast --------- Co-authored-by: Qiyun Dai <[email protected]> commit 5e2dd4c Author: Mariia Lukianets <[email protected]> Date: Wed Jun 26 14:01:10 2024 +0200 MWPW-152283: [TEST] switch WCS to acom domain (#2504) * MWPW-152283: add acom WCS domain option * update pr comments commit 7faf121 Author: Mariia Lukianets <[email protected]> Date: Wed Jun 26 14:01:03 2024 +0200 MWPW-153068: fix missing 'P3Y' in ost (#2502) commit 8b46dea Author: spandit22 <[email protected]> Date: Wed Jun 26 02:16:31 2024 -0700 MWPW-146782: Add Description to Article richresults (#2505) commit 7971880 Author: Nicolas Peltier <[email protected]> Date: Wed Jun 26 11:16:24 2024 +0200 MWPW-146326 introduce promo text for all cards (#2483) * MWPW-146326 refactor merch-card unit tests * MWPW-146326 introduce promo text for all merch cards except special offer cards. Also get rid of Devices text in slot['detail-m'] as per ticket decision * MWPW-146326 first set of feedbacks * MWPW-146326 removing only * MWPW-146326 some more changes * MWPW-146326 fixed new lint issues * MWPW-146326 make card UT more talkative * MWPW-146326 rename TEXT_STYLES * MWPW-146326 last changes * MWPW-146326 final commit from mas main commit 9e9751e Author: Jacky Sun <[email protected]> Date: Wed Jun 26 02:16:17 2024 -0700 MWPW-150705: De-couple Result Spreadsheet for Interactive Front Door Experience (#2481) * MWPW-147601: sending ML field text to Analytics * MWPW-147989 and MWPW-151786: handle localized path for quiz spreadsheets and doc files * update code according to feedbacks * MWPW-150705: De-couple Result Spreadsheet for Interactive Front Door Experience * add unit tests * fix lint errors * update lana with tags commit 95837b4 Author: Vivian A Goodrich <[email protected]> Date: Wed Jun 26 03:16:10 2024 -0600 MWPW-152627 [MILO][MARTECH] add Target propositionDisplay call (#2477) * Martech codeupdate (#2476) codeupdate Co-authored-by: Kanda S <[email protected]> * move dangle lint disable to entire file * add empty array backup value for propositions --------- Co-authored-by: Kanda S <[email protected]> Co-authored-by: Kanda S <[email protected]> commit 75d922e Author: Jan Ivan Viloria <[email protected]> Date: Tue Jun 25 15:44:13 2024 -0400 MWPW-143637 [Milo]Component Enhancements for ACE0861 Prioritized Placements HP Test (#2506) * Support button.fill w/ <strong><em> authoring * Initial checkin. * Image centering fix. * 04-02-22 ivan updated milo ace0861 first * Left align block fix. * Gap fixes. * Rollback decorate buttons. * Gap updates. * Button updates. * Remove .brick class. * 06-7-24 ivan change brick css * 06-11-24 ivan remove black button css * 06-25-24 ivan resolve filepath for sticky-section * update brick.js to import handleObjectFit like current stage code * remove unneeded comments * add in section-metadata CSS from commits since we branched off --------- Co-authored-by: thi64146 <[email protected]> Co-authored-by: AdobeLinhart <[email protected]> Co-authored-by: vgoodric <[email protected]> commit 2fa8a87 Author: Narcis Radu <[email protected]> Date: Tue Jun 25 19:12:01 2024 +0300 [MWPW-129964] Fix accessibility issues for georouting language selector (#2497) commit c05ef86 Author: Brandon Marshall <[email protected]> Date: Tue Jun 25 09:11:54 2024 -0700 MWPW-148424 Fix chart dates (#2466) commit 1a8acc6 Author: Dev Ashish Saradhana <[email protected]> Date: Mon Jun 24 17:47:18 2024 +0530 MWPW-139487 - Fix for Gnav scroll issue with Landscape Orientation on devices (#2473) * gnav scroll issue fix * Fix review comments * Fix review comments * Fix review commets commit 1d3a048 Author: nishantka <[email protected]> Date: Mon Jun 24 14:38:14 2024 +0530 MWPW-151588: fix geo modal is not closed on ESC (#2449) Co-authored-by: milo-pr-merge[bot] <169241390+milo-pr-merge[bot]@users.noreply.github.com> commit 5ad4b21 Author: Mira Fedas <[email protected]> Date: Thu Jun 20 14:16:44 2024 +0200 MWPW-148002: Adjust Strike-through price font size for: merch-card (all variations) (#2459) * descreased strikethrough price font size * use milo style variable for font size --------- Co-authored-by: Ilyas Stéphane Türkben <[email protected]> commit 203c3e6 Author: Dev Ashish Saradhana <[email protected]> Date: Thu Jun 20 15:12:28 2024 +0530 MWPW-140523: Update existing LANA logs to capture potential client errors (#2402) * Add missing LANA logs for import and fetch * Added lana logs * fix lint issues * Fix review comments * Fix eslint issues * Fix Review comment * Fix lint issue * Fix review comments * Update UT for the changes * Fix lint commit ce3764f Author: Narcis Radu <[email protected]> Date: Thu Jun 20 12:15:17 2024 +0300 [MWPW-152676] Ensure all Graybox URLs use STAGE environment (#2488) commit 75fe20d Author: Ryan Clayton <[email protected]> Date: Wed Jun 19 08:24:30 2024 -0600 [MWPW-137401] Pathfinder origin fix (#2464) * [MWPW-137401] Pathfinder origin fix Fixes Sharepoint origin for consumers not on same SharePoint site as Milo. Resolves: MWPW-137401 * Address unit tests --------- Co-authored-by: Ryan Clayton <[email protected]> commit 5850d08 Author: Mira Fedas <[email protected]> Date: Wed Jun 19 16:24:22 2024 +0200 MWPW-148054: Mnemonics list in a text block (#2405) * integrated mnemonics into text block * used loadBlock, removed unneccessary checking * added unit test for mnemonics in text block commit 4bda3a5 Author: Vivian A Goodrich <[email protected]> Date: Wed Jun 19 08:24:15 2024 -0600 MWPW-152659 [MILO][MODAL][ANALYTICS] Analytic on modal load is missing (#2478) * add check before overriding digitalData * fix object creation to avoid erro commit 0428594 Author: Ryan Parrish <[email protected]> Date: Tue Jun 18 04:37:50 2024 -0600 MWPW-144801 - [Hero-Marquee] 🆕 block (#2412) * initial marquee-hero block * allow key w/ classes structure * button-l default * rename lockup class to flex-row * added icon-list item types * Enable meta rows be authored above main content && authored Icon list support * update block name to just hero and row keys to hero- * added textOverrides * support for 'reverse, reverse-mobile' and remove empty split asset div * fixed padding on split mobile views, linting * multi classes on key rows. Cleaned up some format * decorateBg, order-reverse, con-block dark/light tablet, desktop selectors * default copy margins, extended decorateText for target * updated block name to hero-marquee, per GWP reques, renamed row key to 'milo-row-type` * decorateBgRow fix for selector * Fixed icon-size inheritance across rows and block variant * lockup default * Extending button class in row-text types * min-height on tablet/desk * trim badge titles and set approp margin * Few design feedback items, removed badge * Default spacing on bg-stack * Few design feedback refeinments * added tests - `con-block-row-` naming - Static links w/ dark-tablet etc. - min-height-m (560), min-height-l (700), m-height-l-desktop, etc... - `5 | 7` vs `6 | 6` - Did this w/ 100 margin-end on .copy in desktop. rather then grid 12. Easier w/ split set up - spacing w/ bg-stack-mobile etc - didn't force padding on type. Use authoring instead - bullet icon size margin - OK as is, if bullet and icon are used together the :marker: is default margin * fix align items action area * no default padding on bg-stack * refine naming conventions * Fixed decorate text logic to not rely on buttonClass to continue * distill sup content * bgcolor naming * bgcolor Type * con-vars support for bg-stack-* * - Split image display cover and 0 padding on stacked mobile/tablet. top or bttm 0 depending on stacking order - Split w/ video - treat same as image * milo_blocks list * split variant default padding bottom * added block to adobetv list * updated test row type -row-bgcolor * typo in class * moved bg-stack out of styles.css and scoped to hero, shared bg posltion feature from bricks and made a global export, added bg-stack-bottom-mobile to set order of that feature * logic style for split-asset * refactor asset selector for autoblock content * updated naming convention of fg-media * refactored .split to .fg-media-cover && .foreground-split to .foreground-media * fg-media-top -> fg-media-stack-top. same as bg-stack * fixed fg-media-cover sel to fix poly elem * refactored min-height-l, to l-min-height for naming convention * addressed a few rtl lang displays * updated icons to use margin-inline for better rtl support. Added selector for .image-link as asset * moved breakpoint-thheme into own file and imported based on classFind * decorateBtns by defatult on row-text type * few missing selectors in bg-stack, removed order .action-area defatul * fixed scope of .fg-media .foreground * added -only to tablet/desktop breakpoint-theme * fixed scope of fg-stack-top * distill CLass in -list, fg-media-hidden-mobile w/ foreground-asset * padding fix for media-hidden * lockup transform * padding on bg-stack-bottom * default min-height, helper classes tablet/desktop * -min-height defaults * clean up a few stylss * rtl fix for foreground-media * code coverage * removed blocks/marquee-hero - fragments of the past * Few nit PR formatting feeback issues resolve * Few PR feedback refactors * minor css optimizing * fixed some linting issues and optimized selectors * refined the breakpoint-theme a bit w/ :is scopes * revert style.css * added conditional for distillClasses, removed if w/ switch * removed variant names `fg-` and `stack-` * bg-top, vs bg-stack * linting * removed shorthand `mobile-tablet` vp selectors * check rtl support fix --------- Co-authored-by: milo-pr-merge[bot] <169241390+milo-pr-merge[bot]@users.noreply.github.com> commit 16c64e4 Author: ilyas Stéphane Türkben <[email protected]> Date: Mon Jun 17 18:10:48 2024 +0200 Container PR for initial TWP features (#2407) * Mwpw-136871: TwP merch card (#2088) * MWPW-136871: TwP Merch Card * unit tests * TWP block draft (#2105) * add draft for a merch-twp block --------- Co-authored-by: Axel Cureno Basurto <[email protected]> * Mwpw 136871: Price display inside TwP merch card (#2110) * MWPW-136871: TwP Merch Card * unit tests * twp card * price display in TwP * deps update * Mwpw 136871 (#2154) * MWPW-136871: TwP Merch Card * unit tests * twp card * price display in TwP * deps update * MWPW-138927: merch-twp WIP (#2160) Merging proactively for a GWP demo. * MWPW-138927: desktop step 2 layout WIP * remove comment * pr feedback * update deps * update deps * update deps * update deps * fixed tests * fix lagging subscription panel * WIP * fix tests * fix tests * wip * fix failing test * fix failing tests * remove files * improve code coverage * address first batch of comments * fix dependencies * fix unit tests * add missing test cases --------- Co-authored-by: Axel Cureno Basurto <[email protected]> Co-authored-by: Mariia Lukianets <[email protected]> Co-authored-by: Axel Cureno Basurto <[email protected]> commit 82b5a83 Author: Okan Sahin <[email protected]> Date: Mon Jun 17 14:16:56 2024 +0200 mwpw-152624: improve stage PR communication (#2474) mwpw-152624: improve stage communication commit 4e8e74c Author: Rares Munteanu <[email protected]> Date: Mon Jun 17 11:23:43 2024 +0200 [MWPW-151172][Kodiak] Package updates (#2467) * [MWPW-151172] Package updates * [MWPW-151172] Update dependencies; add zero-impact file commit 112b462 Author: Jason Slavin <[email protected]> Date: Mon Jun 17 02:15:19 2024 -0700 MWPW-148282: Remove data layer names from Marketo Configurator (#2461) * Removing data layer descriptions from fields * removing console.log commit b5d1b31 Author: Nicolas Peltier <[email protected]> Date: Mon Jun 17 11:15:12 2024 +0200 MWPW-147206 making promo schedule geo aware (#2438) * MWPW-147206 making promo schedule geo aware mostly to allow authors enabling promos in separate smaller files, schedule directive has now a locales setting to narrow its activation to those countries promoEnabled parameter passed to getPromoManifests in promo utils is now an object that has entries for each region promo name that are fetched at very early time (no geo resolution), default being kept for backward compatibility reasons with key `manifestnames`. getPromoManifests then resolves current locale and serves merged manifests between regional specific ones (if locale matches), and 'global' ones. * lint & ut fix * MWPW-147206 removing non matching manifests * lint fix * review fixes * MWPW-147206 rares changes commit 66ea37f Author: Raghav Sharma <[email protected]> Date: Mon Jun 17 14:45:05 2024 +0530 MWPW-142935 [Project PEP] PEP <> Geo Routing Modal Interactions (#2403) * modified georouting and pep so that pep only appears if a) GRM is not shown or b) GRM is dismissed * Unit test for GRM/PEP interaction * changed grmActive to geoRoutinActive * Fixed unit test for grm/pep interaction * removed unused imports * Cleaned up the implementation * Changed from polling to listening for an event * removed an unnecessary line * Modified unit test * Changed the implementation to use polling for any modal * Changed pep loading logic to poll for modals only after the close modal dialog comes through commit 3221e37 Author: Axel Cureno Basurto <[email protected]> Date: Mon Jun 17 11:14:57 2024 +0200 MWPW-146856: Left/right padding missing on segment cards on mobile (#2229) * MWPW-146856: Left/right padding missing on segment cards on mobile Fixes Left/right padding missing on segment cards on mobile viewport. Tacocat PR: https://git.corp.adobe.com/wcms/tacocat.js/pull/584 Resolves: MWPW-146856 Test URLs: Before: https://main--milo--adobecom.hlx.page/?martech=off After: https://MWPW-146856--milo--adobecom.hlx.page/?martech=off * Update merch-card.js * Update merch-card.js * Update merch-card.js * updated dependency * Update merch-card.js commit 0cdd84a Author: Sean Archibeque <[email protected]> Date: Thu Jun 13 10:08:54 2024 -0600 MWPW-151885 - [LocUI] messaging for auth redirect (#2463) * foe locui block that displays the message to re-run * update message commit 083f2f1 Author: Ryan Clayton <[email protected]> Date: Thu Jun 13 10:08:47 2024 -0600 [MWPW-151673] Library templates (#2455) * [MWPW-151673] Library templates Add templates to sidekick library Resolves: MWPW-151673 * Fixing library unit tests --------- Co-authored-by: Ryan Clayton <[email protected]> commit 96ba26c Author: Mark Perry <[email protected]> Date: Thu Jun 13 09:08:40 2024 -0700 MWPW-151790[MILO][MEP] Organize getPersConfig function in personalization.js (#2454) * replace overridePersonalizationVariant function * Fix chaining * Move parseMepParam * add additional optional chaining operator Co-authored-by: Vivian A Goodrich <[email protected]> * Refactor personalization.js * Remove comment * Add back in deleted variants --------- Co-authored-by: Vivian A Goodrich <[email protected]> commit 2614acd Author: Okan Sahin <[email protected]> Date: Thu Jun 13 18:08:33 2024 +0200 Increase 404 rum tracking to 10% (#2448) * Increase 404 rum tracking to 10% * Address feedback * Address feedback * Increase the general tracking weight for 404 pages * Increase tracking for 404s * Improve ID creation * Use getMetadata commit 3d61c97 Author: Vivian A Goodrich <[email protected]> Date: Wed Jun 12 14:09:43 2024 -0600 MWPW-152456 [MILO][MEP] using the `not-q-always-on-promo` entitlement is selected when it should not be (#2469) add space commit e10cd54 Author: Chris Peyer <[email protected]> Date: Wed Jun 12 16:09:36 2024 -0400 MWPW-140906 Graybox Misc UI Fixes (#2458) * Graybox bug fixes * Support both gb-no-click and no-click * no-click and gnav fixes * device preview fixes * Fix and add tests commit 5a932b9 Author: Mark Perry <[email protected]> Date: Tue Jun 11 12:11:41 2024 -0700 MWPW-148437 [MILO][MEP] Placeholders don't work if you qualify for default and override with mep= (#2439) * replace overridePersonalizationVariant function * Fix chaining * Move parseMepParam * add additional optional chaining operator Co-authored-by: Vivian A Goodrich <[email protected]> * MWPW-151561 [MILO][MEP] MEP button does not appear if Target is on but there are no pzn manifests (#2441) * calculate preview in utils * update personalization to not recalculate * replace overridePersonalizationVariant function * Fix chaining * Move parseMepParam * add additional optional chaining operator Co-authored-by: Vivian A Goodrich <[email protected]> * MWPW-151561 [MILO][MEP] MEP button does not appear if Target is on but there are no pzn manifests (#2441) * calculate preview in utils * update personalization to not recalculate * Revert "MWPW-151561 [MILO][MEP] MEP button does not appear if Target is on but there are no pzn manifests (#2441)" This reverts commit eca39b0. --------- Co-authored-by: Vivian A Goodrich <[email protected]> Co-authored-by: vgoodric <[email protected]> commit eeca38e Author: Sanjay Rai <[email protected]> Date: Tue Jun 11 12:11:34 2024 -0700 MWPW-147594: Consumers mappings for origin not being sent when using sidekick (#2413) * Actually fixing original bug in sidekick * Increasing code coverage * Fixing missing export --------- Co-authored-by: milo-pr-merge[bot] <169241390+milo-pr-merge[bot]@users.noreply.github.com> commit af819cd Author: Jason Slavin <[email protected]> Date: Tue Jun 11 12:11:27 2024 -0700 MWPW-148278: Dynamic-Nav hide breadcrumbs (#2380) * Adding feature to hide breadcrumbs when dynamic-nav is active * Moving logic for 'has-breadcrumbs' into utils.js to prevent CLS * Removing extraneous code from gnav since utils modifies breadcrumb functionality * Adding feature to hide breadcrumbs when dynamic-nav is active * Moving logic for 'has-breadcrumbs' into utils.js to prevent CLS * Removing extraneous code from gnav since utils modifies breadcrumb functionality commit afe81dc Author: Mark Perry <[email protected]> Date: Tue Jun 11 12:11:20 2024 -0700 MWPW-152041[MEP][MILO] Add support for using MEP placeholders in gnav (#2446) Fix gnav placeholders commit f5e66e0 Author: Blaine Gunn <[email protected]> Date: Mon Jun 10 08:18:22 2024 -0600 MWPW-147299 How-to MP4 Videos (#2411) commit e63812e Author: Okan Sahin <[email protected]> Date: Mon Jun 10 16:01:46 2024 +0200 mwpw-150087: bump action dependencies (#2451) * mwpw-150087: bump action dependencies * bump codeql * bump paths filter * Update codecov * Remove fixed versions commit e5cb173 Author: Okan Sahin <[email protected]> Date: Mon Jun 10 12:42:55 2024 +0200 mwpw-151992: allow to always merge zero impact PRs (#2436) * mwpw-151992: allow to always merge zero impact PRs * Label zero impact PRs in slack & PR description commit 537eec4 Author: Nicolas Peltier <[email protected]> Date: Mon Jun 10 11:16:40 2024 +0200 MWPW-146254 make merch quantity input a textfield for ff (#2444) commit 5dfb72a Author: Mariia Lukianets <[email protected]> Date: Mon Jun 10 11:16:33 2024 +0200 MWPW-146963: OST to support Annual template (#2431) * consume new ost * upd to latest * test ost changes * support annual merch link * MWPW-146963: calculate annual of ABM * fix review comment * include revert of breaking change commit 5522ba3 Author: Narcis Radu <[email protected]> Date: Mon Jun 10 12:16:27 2024 +0300 [MWPW-151474] Ensure carousel navigation is displayed after everything has been loaded (#2427) [MWPW-151474] Ensure navigation is displayed after everything has been loaded commit bdf4368 Author: Ruchika Sinha <[email protected]> Date: Mon Jun 10 02:16:20 2024 -0700 MWPW-151281-Fix overflow causing horizontal scroll (#2421) * Update section-metadata.css * fix indentation commit 782ba81 Author: Rares Munteanu <[email protected]> Date: Mon Jun 10 11:16:12 2024 +0200 [MWPW-150668] Get utils ready for Helix 5 (#2376) commit 45c1c3c Author: Okan Sahin <[email protected]> Date: Mon Jun 10 11:16:06 2024 +0200 Add news.adobe.com as known federal origin (#2375) Add news.adobe.com as trusted federal origin commit 366c1fe Author: Jacky Sun <[email protected]> Date: Mon Jun 10 02:15:59 2024 -0700 MWPW-147989 and MWPW-151786: handle localized path for quiz spreadsheets and doc files (#2416) * MWPW-147601: sending ML field text to Analytics * MWPW-147989 and MWPW-151786: handle localized path for quiz spreadsheets and doc files * update code according to feedbacks commit ca8ba03 Author: Mira Fedas <[email protected]> Date: Thu Jun 6 18:11:18 2024 +0200 MWPW-147267: Mini Compare Chart Card UX Feedback (#2320) avoided adding empty p in slots when there is no content commit 1c36748 Author: Okan Sahin <[email protected]> Date: Thu Jun 6 13:57:24 2024 +0200 [MWPW-151992] Resolve permission issues for the zero impact workflow (#2435) Run workflow on pr targets commit d6f22ce Author: Vivian A Goodrich <[email protected]> Date: Thu Jun 6 03:15:16 2024 -0600 MWPW-147741 [Geo-Routing] Need to load modal after bootstrap in order to ensure Analytics fires (#2399) * move geomodal creation * Add initial switch * switch to daa-ll * pulling recs from vhargrave * roll back utils * remove await for martech in utils, add check in modals.js * remove unrelated changes * unit test coverage * add in check for called and resets on called --------- Co-authored-by: markpadbe <[email protected]> commit 95a6593 Author: Okan Sahin <[email protected]> Date: Wed Jun 5 14:42:42 2024 +0200 Start the RCP for stage earlier (#2373) Add offset to stage merges commit 7e0a838 Author: Robert Bogos <[email protected]> Date: Wed Jun 5 15:42:15 2024 +0300 [MWPW 141389] - Adding more icons to the SideKick icon list (#2422) * test new icon * added add icon * added more icons * added ai generate icon * hotfix * hotfix * organised icons in alphabetical order * formatting * formatting * formatting commit 9b736bb Author: Robert Bogos <[email protected]> Date: Wed Jun 5 15:17:42 2024 +0300 Fixed failing unit tests (#2426) fixed failing unit tests commit fa9f191 Author: Denys Fedotov <[email protected]> Date: Wed Jun 5 03:03:08 2024 -0600 MWPW-146577: [Kodiak]: Cross-site Scripting (XSS) in JavaScript in [github.com/adobecom/milo] (#2384) * Removed DOMXSS vulnerability noticed by Kodiak * lint errors fix * removed txt version of the div * using createTag instead of document.createElement * lint error * lint error * refactored to simplify * lint --------- Co-authored-by: Denys Fedotov <[email protected]> commit c500143 Author: Robert Bogos <[email protected]> Date: Wed Jun 5 12:03:01 2024 +0300 [MWPW-146056] Links are not converted to their stage counterparts (#2361) * test fix * stage env - replace hostname on decorating links * stage env - replace hostname on decorating links * hotfix * added stage domains mapping for links conversion * hotfix * consumer side stage domains mapping for links conversion * hotfix * extended unit tests commit fe0adb4 Author: Ryan Parrish <[email protected]> Date: Tue Jun 4 04:50:39 2024 -0600 MWPW-142084 - Accessibility: ARIA attribute needed on "Play" button for Image video links (#2398) * added conditional and style for items center * remove console * add area-label: play to image-video-link * removed area-label from play-icon-container commit 01a9b19 Author: Ruchika Sinha <[email protected]> Date: Tue Jun 4 03:34:44 2024 -0700 MWPW-130601-Add typekit for adobe clean thai (#2389) * Update styles.css * Update styles.css * Update thai typekit * Update scripts.js commit 332ce79 Author: sharathkannan <[email protected]> Date: Tue Jun 4 16:04:36 2024 +0530 fix(MWPW-148021):Table min width set to screen size. (#2360) Table min width set to screen width commit 2993e24 Author: Brad Johnson <[email protected]> Date: Tue Jun 4 05:34:29 2024 -0500 MWPW-147770 Mobile Touch (#2356) MWPW-147770 added mobile touch/swipe handling commit 1362ce1 Author: Narcis Radu <[email protected]> Date: Tue Jun 4 13:34:23 2024 +0300 [MWPW-141785] Implement custom link actions (#2324) Co-authored-by: Okan Sahin <[email protected]> commit a60b132 Author: Vivian A Goodrich <[email protected]> Date: Mon Jun 3 14:09:24 2024 -0600 MWPW-151430 [MILO][MEP] not in front of a segment does not work (#2401) * rename * remove extra space
showModal
function)setTimeout
will add the rechecking function to the event queue). Once the modal is no longer in the DOM, PEP's init function will fireResolves: MWPW-142935
Test URLs: